[Sema/SILGen/IRGen/StdLib] Implement metatype keypaths #73242

amritpan · 2024-04-25T02:59:38Z

This is the implementation for SE-0438: Metatype Keypaths which extends keypath usage to include references to static properties, eg. metatype keypaths:

struct Bee {
  static let name = "honeybee"
}

let kp = \Bee.Type.name

xedin · 2024-04-30T17:00:00Z

test/ModuleInterface/static_keypaths.swift

+// CHECK: true
+print(keyPath3FromLibB == keyPath3FromLibC)
+// CHECK: true
+print(keyPath3FromLibB != keyPath4FromLibC)


@amritpan I think we need a lot more tests. Could you please add tests that access static properties and subscripts to test/SILGen/keypaths.swift, we also need chained access tests in Sema and in SILGen and interpreter tests for all of the above that make sure that changes behave as expected at runtime.

test/expr/unary/keypath/keypath.swift

lib/Sema/CSGen.cpp

lib/Sema/CSFix.cpp

include/swift/Sema/CSFix.h

xedin · 2024-05-01T17:53:46Z

lib/SIL/Verifier/SILVerifier.cpp

@@ -7192,6 +7192,8 @@ void SILProperty::verify(const SILModule &M) const {
  auto sig = dc->getGenericSignatureOfContext();
  auto baseTy = dc->getInnermostTypeContext()->getSelfInterfaceType()
                  ->getReducedType(sig);
+  if (decl->isStatic())
+    baseTy = CanMetatypeType::get(baseTy);


Since retrieval of the base type is not as straightforward anymore I think it should either become a property of SILProperty or we should add a method on SILProperty i.e. CanType SILProperty::getBaseType() const to compute it internally.

I added CanType SILProperty::getBaseType() const to SILVerifier.cpp above ::verify and don't think that is the place for it but am not sure where else it should go. Perhaps at the end of SILType.cpp?

xedin · 2024-05-17T21:32:55Z

@swift-ci please build toolchain macOS platform

xedin · 2024-05-28T20:02:56Z

@swift-ci please build toolchain Linux platform

Azoy · 2024-05-30T17:03:08Z

test/abi/Inputs/macOS/arm64/stdlib/baseline-asserts

@@ -1050,6 +1050,7 @@ _$sSP17withMemoryRebound2to8capacity_qd_0_qd__m_Siqd_0_SPyqd__GKXEtKr0_lF
 _$sSP25customPlaygroundQuickLooks01_bcD0Ovg
 _$sSP25customPlaygroundQuickLooks01_bcD0OvpMV
 _$sSP4_maxSPyxGvgZ
+_$sSP4_maxSPyxGvpZMV


You should edit abi/macOS/{arch}/stdlib.swift instead of adding these to the baselines. Also, do we have to emit these property descriptors for every static property? Can we make these conditionally emittable?

Thank you - I've moved them over. And yes, with this implementation, we will need to emit property descriptors for every static property.

Similarly with the other baseline-asserts 😅

Thank you! I've moved the rest over as well.

amritpan · 2024-07-29T23:13:25Z

@swift-ci please build toolchain macOS platform

amritpan · 2024-09-26T03:45:24Z

@swift-ci please test

amritpan · 2024-09-27T05:39:07Z

@swift-ci please test

groue · 2024-09-27T10:13:29Z

Glad to see this long-awaited feature is in progress :-)

amritpan · 2024-09-27T18:33:54Z

@swift-ci please test

amritpan · 2024-09-28T05:44:21Z

@swift-ci please smoke test

slavapestov · 2024-09-30T03:41:25Z

It looks like some of the tests now need to be marked REQUIRES: asserts, because they currently fail in a noassert build:

<unknown>:0: error: experimental feature 'KeyPathWithStaticMembers' cannot be enabled in production compiler

Instead, it would be better to split off the tests for the new feature into their own file(s).

xedin · 2024-09-30T05:02:24Z

@amritpan could you please all the tests where you added the flat? Looks like we didn’t get all of them

shahmishal · 2024-10-02T06:57:56Z

We are seeing swift macOS toolchain bot failing most likely related to this PR.

FAIL: Swift(macosx-x86_64) :: abi/macOS/x86_64/stdlib.swift (9366 of 18223)
******************** TEST 'Swift(macosx-x86_64) :: abi/macOS/x86_64/stdlib.swift' FAILED ********************
Script:
--
: 'RUN: at line 1';   rm -rf "/Users/ec2-user/jenkins/workspace/oss-swift-package-macos/build/buildbot_osx/swift-macosx-x86_64/test-macosx-x86_64/abi/macOS/x86_64/Output/stdlib.swift.tmp" && mkdir -p "/Users/ec2-user/jenkins/workspace/oss-swift-package-macos/build/buildbot_osx/swift-macosx-x86_64/test-macosx-x86_64/abi/macOS/x86_64/Output/stdlib.swift.tmp"
: 'RUN: at line 2';   /Users/ec2-user/jenkins/workspace/oss-swift-package-macos/build/buildbot_osx/llvm-macosx-x86_64/bin/llvm-nm -g --defined-only -f just-symbols /Users/ec2-user/jenkins/workspace/oss-swift-package-macos/build/buildbot_osx/swift-macosx-x86_64/lib/swift/macosx/x86_64/libswiftCore.dylib > /Users/ec2-user/jenkins/workspace/oss-swift-package-macos/build/buildbot_osx/swift-macosx-x86_64/test-macosx-x86_64/abi/macOS/x86_64/Output/stdlib.swift.tmp/symbols
: 'RUN: at line 3';   /Applications/Xcode-beta.app/Contents/Developer/usr/bin/python3 /Users/ec2-user/jenkins/workspace/oss-swift-package-macos/swift/utils/swift-abi-symbol-checker.py /Users/ec2-user/jenkins/workspace/oss-swift-package-macos/swift/test/abi/macOS/x86_64/stdlib.swift /Users/ec2-user/jenkins/workspace/oss-swift-package-macos/build/buildbot_osx/swift-macosx-x86_64/test-macosx-x86_64/abi/macOS/x86_64/Output/stdlib.swift.tmp/symbols
: 'RUN: at line 4';   diff -u /Users/ec2-user/jenkins/workspace/oss-swift-package-macos/swift/test/abi/macOS/x86_64/../../Inputs/macOS/x86_64/stdlib/baseline /Users/ec2-user/jenkins/workspace/oss-swift-package-macos/build/buildbot_osx/swift-macosx-x86_64/test-macosx-x86_64/abi/macOS/x86_64/Output/stdlib.swift.tmp/symbols
--
Exit Code: 1

Command Output (stdout):
--
--- /Users/ec2-user/jenkins/workspace/oss-swift-package-macos/swift/test/abi/macOS/x86_64/../../Inputs/macOS/x86_64/stdlib/baseline	2024-10-02 03:37:11
+++ /Users/ec2-user/jenkins/workspace/oss-swift-package-macos/build/buildbot_osx/swift-macosx-x86_64/test-macosx-x86_64/abi/macOS/x86_64/Output/stdlib.swift.tmp/symbols	2024-10-02 06:19:33
@@ -1,4 +1,8 @@
 $ld$previous$@rpath/libswiftCore.dylib$$1$10.9$10.14.4$$
+<MISSING ADDITION> _$ss24_RuntimeFunctionCountersV03numabC0SivpZMV
+<MISSING ADDITION> _$ss24_RuntimeFunctionCountersV07runtimeB11NameToIndexSDySSSiGvpZMV
+<MISSING ADDITION> _$ss24_RuntimeFunctionCountersV07runtimeB5NamesSaySSGvpZMV
+<MISSING ADDITION> _$ss24_RuntimeFunctionCountersV07runtimebC7OffsetsSPys6UInt16VGvpZMV
 _$s11MaskStorages4SIMDPTl
 _$s11RawExponentSBTl
 _$s11SubSequenceSlTl

--

********************

xedin · 2024-10-02T08:44:59Z

@amritpan could you please take a look?

amritpan · 2024-10-02T17:13:23Z

@shahmishal I've opened this PR to resolve this. Thank you!

shahmishal · 2024-10-02T17:14:54Z

@slavapestov Created this PR: #76824

Originally added in swiftlang#73242. swiftlang#76826 was up to remove them but we took swiftlang#76824 instead. Resolves rdar://136918801.

amritpan · 2024-10-04T08:33:14Z

@shahmishal It seems that the abi test failures are persisting? I've reopened #76826.

amritpan requested review from hborla, slavapestov, xedin and a team as code owners April 25, 2024 02:59

amritpan changed the title ~~Implement metatype keypaths~~ [Sema/SILGen/IRGen/StdLib] Implement metatype keypaths Apr 25, 2024

Azoy requested a review from jckarter April 25, 2024 16:40

amritpan force-pushed the metatype-kp-implementation branch from a1b2bb5 to 0ec01dc Compare April 25, 2024 22:27

xedin reviewed Apr 30, 2024

View reviewed changes

test/expr/unary/keypath/keypath.swift Show resolved Hide resolved

xedin reviewed May 1, 2024

View reviewed changes

amritpan force-pushed the metatype-kp-implementation branch 3 times, most recently from bc8d04b to 71bc3a6 Compare May 5, 2024 01:51

Azoy reviewed May 30, 2024

View reviewed changes

amritpan force-pushed the metatype-kp-implementation branch 4 times, most recently from 96fda00 to ebd7dfc Compare June 10, 2024 00:25

amritpan force-pushed the metatype-kp-implementation branch from ebd7dfc to bd773b7 Compare July 29, 2024 23:12

amritpan requested review from a team, eeckstein, xymus and rjmccall as code owners July 29, 2024 23:12

amritpan requested a review from xedin July 29, 2024 23:13

amritpan force-pushed the metatype-kp-implementation branch from bd773b7 to 5e4b371 Compare August 28, 2024 21:56

amritpan force-pushed the metatype-kp-implementation branch from 5e4b371 to ed098bb Compare September 6, 2024 20:57

amritpan force-pushed the metatype-kp-implementation branch from ed098bb to 5ed66f1 Compare September 21, 2024 20:03

amritpan force-pushed the metatype-kp-implementation branch from 162cdaa to 6778a34 Compare September 26, 2024 03:45

amritpan force-pushed the metatype-kp-implementation branch from 6778a34 to 4e63e02 Compare September 27, 2024 05:36

amritpan force-pushed the metatype-kp-implementation branch from 4e63e02 to 1865936 Compare September 27, 2024 18:33

amritpan force-pushed the metatype-kp-implementation branch from 1865936 to fa51809 Compare September 28, 2024 05:39

amritpan added 6 commits September 27, 2024 22:42

[Tests] Update Interpreter tests.

6a6e6ee

[Tests] Update Sema tests.

2c641a8

[Tests] Update SilGen tests.

605fe8f

[Tests] Update ABI tests.

c5ebf4e

[Tests] Add static keypaths tests.

c69e5cb

Guard feature behind experimental flag.

8ebc928

amritpan force-pushed the metatype-kp-implementation branch from fa51809 to 8ebc928 Compare September 28, 2024 05:42

amritpan merged commit 6066418 into swiftlang:main Sep 28, 2024
3 checks passed

amritpan deleted the metatype-kp-implementation branch September 28, 2024 09:46

amritpan mentioned this pull request Sep 30, 2024

[Test] Mark additional tests with REQUIRES: asserts for KeyPathWithStaticMembers flag. #76776

Merged

amritpan mentioned this pull request Oct 2, 2024

[Test] Remove abi tests descriptors for KeyPathWithStaticMembers flag. #76826

Merged

bnbarham added a commit to bnbarham/swift that referenced this pull request Oct 3, 2024

Remove symbols that are no longer part of ABI

030298c

Originally added in swiftlang#73242. swiftlang#76826 was up to remove them but we took swiftlang#76824 instead. Resolves rdar://136918801.

bnbarham mentioned this pull request Oct 3, 2024

Remove symbols that are no longer part of ABI #76854

Closed

bnbarham added a commit to bnbarham/swift that referenced this pull request Oct 3, 2024

Remove symbols that are no longer part of ABI

cee41ef

Originally added in swiftlang#73242. swiftlang#76826 was up to remove them but we took swiftlang#76824 instead. Resolves rdar://136918801.

[Sema/SILGen/IRGen/StdLib] Implement metatype keypaths #73242

[Sema/SILGen/IRGen/StdLib] Implement metatype keypaths #73242

Uh oh!

Conversation

amritpan commented Apr 25, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

xedin Apr 30, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

xedin May 1, 2024

Choose a reason for hiding this comment

Uh oh!

amritpan May 3, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xedin commented May 17, 2024

Uh oh!

xedin commented May 28, 2024

Uh oh!

Azoy May 30, 2024

Choose a reason for hiding this comment

Uh oh!

amritpan Jun 5, 2024

Choose a reason for hiding this comment

Uh oh!

Azoy Jun 6, 2024

Choose a reason for hiding this comment

Uh oh!

amritpan Jun 10, 2024

Choose a reason for hiding this comment

Uh oh!

amritpan commented Jul 29, 2024

Uh oh!

amritpan commented Sep 26, 2024

Uh oh!

amritpan commented Sep 27, 2024

Uh oh!

groue commented Sep 27, 2024

Uh oh!

amritpan commented Sep 27, 2024

Uh oh!

amritpan commented Sep 28, 2024

Uh oh!

Uh oh!

slavapestov commented Sep 30, 2024

Uh oh!

xedin commented Sep 30, 2024

Uh oh!

shahmishal commented Oct 2, 2024

Uh oh!

xedin commented Oct 2, 2024

Uh oh!

amritpan commented Oct 2, 2024

Uh oh!

shahmishal commented Oct 2, 2024

Uh oh!

amritpan commented Oct 4, 2024

Uh oh!

Uh oh!

amritpan commented Apr 25, 2024 •

edited

Loading

amritpan May 3, 2024 •

edited

Loading